refactor(Bowl): remove IPC dependency, simplify API, drop macOS#491
Merged
Conversation
Breaking changes:
- Remove Bowl 4-param internal constructor; only 1 public paramless constructor
- Remove Bowl.CreateParameter() and Bowl.MapToContext() (IPC-based entry points)
- Remove SetVariable('UpgradeFail') step from crash pipeline (was auto-deleted on read)
- BowlContext no longer depends on MonitorParameter
- Crash report JSON uses flat fields instead of nested MonitorParameter
Removed modules:
- Ipc/ (Environments, IpcEncryption) — duplicate of Core's IPC
- Configuration/ProcessContract — Bowl-specific, unused without IPC
- Internal/EnvironmentProvider (IEnvironmentProvider) — no longer needed
- Internal/LinuxSystem — only used by deleted LinuxStrategy
- Strategies/MacBowlStrategy — lldb unusable in production (SIP/task_for_pid)
- Strategys/ (MonitorParameter, AbstractStrategy, IStrategy, WindowStrategy,
LinuxStrategy) — legacy synchronous API, all [Obsolete]
Linux improvements:
- Check PATH for procdump before attempting sudo install
- Auto-detect PID (-p) vs process name (-w) for procdump flag
- Write bowl_linux_unsupported.txt in fail directory for unsupported distros
API surface cleanup:
- TextTraceListener, WindowsOutputDebugListener: public -> internal
- StorageHelper: public -> internal
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors GeneralUpdate.Bowl to remove the AES-encrypted IPC dependency between Bowl and Core, simplifies the public API by replacing MonitorParameter/IPC-based entry points with a BowlContext value type, drops the macOS strategy (since lldb/task_for_pid is unusable in production), and tightens the public surface to internal for several helper types. The Crash JSON now uses flat fields, and the Linux strategy gains a which procdump pre-check, PID/name auto-detection (-p vs -w), and an bowl_linux_unsupported.txt diagnostic hint.
Changes:
- Remove IPC layer (
Ipc/Environments.cs,Ipc/IpcEncryption.cs),IEnvironmentProvider/EnvironmentProvider,ProcessContract,LinuxSystem, the legacyStrategys/*(MonitorParameter,AbstractStrategy,IStrategy,WindowStrategy,LinuxStrategy), andMacBowlStrategy; removeBowl.MapToContext/Bowl.CreateParameterand theUpgradeFailenv-var step. - Flatten
Crash(was{ Parameter, ProcdumpOutPutLines }) to direct fields and update the JSON serialization context andCrashReporter. - Improve
LinuxBowlStrategy: probe-once_probedflag,IsProcdumpInPathshort-circuit, distro-detect path,-p/-wflag selection, andWriteUnsupportedHint. Remove obsolete xmldoc remarks.
Reviewed changes
Copilot reviewed 41 out of 41 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/c#/GeneralUpdate.Bowl/Bowl.cs | Drop IPC env DI, remove MapToContext/CreateParameter, drop UpgradeFail step, simplify OnCrash path. |
| src/c#/GeneralUpdate.Bowl/BowlContext.cs | Remove obsolete reference to Strategys.MonitorParameter from xmldoc. |
| src/c#/GeneralUpdate.Bowl/Internal/Crash.cs | Flatten fields and document each property. |
| src/c#/GeneralUpdate.Bowl/Internal/CrashReporter.cs | Populate flat Crash fields. |
| src/c#/GeneralUpdate.Bowl/Internal/EnvironmentProvider.cs | Removed (no longer needed). |
| src/c#/GeneralUpdate.Bowl/Internal/LinuxSystem.cs | Removed. |
| src/c#/GeneralUpdate.Bowl/Internal/SystemInfoProviderFactory.cs | Comment updated to drop macOS mention. |
| src/c#/GeneralUpdate.Bowl/Configuration/ProcessContract.cs | Removed. |
| src/c#/GeneralUpdate.Bowl/Ipc/Environments.cs, IpcEncryption.cs | Removed. |
| src/c#/GeneralUpdate.Bowl/Strategys/* | Removed legacy obsolete strategies, interface, abstract base, and MonitorParameter. |
| src/c#/GeneralUpdate.Bowl/Strategies/StrategyFactory.cs | Drop macOS branch. |
| src/c#/GeneralUpdate.Bowl/Strategies/MacBowlStrategy.cs | Removed. |
| src/c#/GeneralUpdate.Bowl/Strategies/LinuxBowlStrategy.cs | Probe procdump in PATH, -p/-w flag selection, write bowl_linux_unsupported.txt. |
| src/c#/GeneralUpdate.Bowl/Tracer/{TextTraceListener,WindowsOutputDebugListener}.cs | Made internal; add namespace to TextTraceListener. |
| src/c#/GeneralUpdate.Bowl/FileSystem/StorageHelper.cs | Made internal. |
| src/c#/BowlTest/* and tests/BowlTest/* | Remove tests for removed types; add BowlContext and Linux-flag/PID tests; rename to English; update Crash flat-field tests; drop MockEnvironmentProvider. |
| src/c#/BowlTest/Strategies/WindowsBowlStrategyTests.cs | Use StartsWith("-e ") instead of Contains(" -e "). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Refactors the
GeneralUpdate.Bowlcomponent to remove internal IPC dependency, simplify the public API, and drop non-functional platform support.Motivation
Bowl previously communicated with Core via AES-encrypted temp files (
Environments/IpcEncryption) to:ProcessInfoto get its config — now replaced by developers passingBowlContextdirectlyUpgradeFailto mark failed versions — now replaced by theOnCrashcallback (developers handle failure marking themselves)The IPC mechanism had a critical bug:
DecryptFromFileauto-deleted the file after read, making theUpgradeFailmark ephemeral (one-read-only). The encrypted file classes were also duplicated verbatim in Core, creating a maintenance burden.macOS strategy (
MacBowlStrategy) was removed becauselldbrequires SIP-disabled /task_for_pidpermissions unavailable in production environments, as the strategy itself documented.Changes
Breaking
Bowlconstructor simplified: 4 params → 1 paramless (internal DI constructor now 3 params)Bowl.CreateParameter()andBowl.MapToContext()removed (IPC-based entry points)SetVariable("UpgradeFail")step removed from crash remediation pipelineCrashreport JSON: flat fields instead of nestedMonitorParameterRemoved (18 files)
Ipc/—Environments.cs,IpcEncryption.cs(duplicate of Core)Configuration/ProcessContract.csInternal/EnvironmentProvider.cs(IEnvironmentProvider)Internal/LinuxSystem.csStrategies/MacBowlStrategy.csStrategys/—MonitorParameter,AbstractStrategy,IStrategy,WindowStrategy,LinuxStrategy(legacy[Obsolete]API)Linux improvements
which procdumpbefore attemptingsudo install.sh-p) vs process name (-w) for procdump flagbowl_linux_unsupported.txtin fail directory explaining why the distro is unsupportedAPI surface cleanup
TextTraceListener,WindowsOutputDebugListener:public→internalStorageHelper:public→internalTesting
🤖 Generated with Claude Code